Repository navigation
ci: pin actions to commit SHAs and run workflows with read-only tokens - #1654
Conversation
Pin every action in the lint, test, and stale workflows to the commit its current tag resolves to, so a moved or compromised tag cannot change what runs. Lint and test get a read-only GITHUB_TOKEN, and their checkouts do not persist the token in the working copy.
MilagrosMarin
left a comment
MilagrosMarin
left a comment
There was a problem hiding this comment.
Thanks @dimitri-yatsenko, this is a clean hardening pass. I checked each pin against its tag via the API, and all eight SHAs match (v4→v4.4.0, v5→v5.6.0, v9→v9.1.0, plus the exact-version pins). Read-only tokens and persist-credentials: false look safe for every job, including hatch-vcs on Windows.
Two small notes, neither blocking:
- #1651 is still open. The description says Dependabot updates are fixed in #1651, but on
masterthe config still hastime: "9:00". Until #1651 lands, these pins won't get bump PRs. Maybe merge #1651 first, or reword to "once #1651 merges". - One transitive tag remains.
pre-commit/action@v3.0.1is a composite action that callsactions/cache@v4by tag, so the lint job still runs one step that isn't pinned. If you want full coverage, the lint job could runpip install pre-commitandpre-commit run ...directly. Otherwise it may be worth softening the "cannot change what runs" wording.
Approving. Let me know if you'd like me to look at the release.yaml pinning follow-up when it's up.
pre-commit/action calls actions/cache by tag and installs an unpinned pre-commit. Run the same steps directly: pre-commit pinned to a version and actions/cache pinned by SHA.
PR 1588 adds a workflow_call trigger just before jobs:, where the permissions block was, so the two PRs conflicted.
|
Thanks @MilagrosMarin. Both notes are addressed:
I also fixed something I'd got wrong in the description. This PR and #1588 did conflict in Could you take another look at these three commits? |
MilagrosMarin
left a comment
MilagrosMarin
left a comment
There was a problem hiding this comment.
Thanks @dimitri-yatsenko, both points are addressed:
✅ Lint no longer goes through pre-commit/action. The written-out steps match the action's v3.0.1 steps (same cache path, key and run flags; only the pip freeze debug step is dropped), and the actions/cache SHA is the commit both v4 and v4.3.0 point to.
✅ #1651 wording updated.
✅ Moving permissions under name: checks out: git merge-tree merges this cleanly with #1588, #1651, #1652 and #1653.
CI is green. Approving.
Summary
This hardens the lint, test, and stale workflows. The versions CI runs do not change.
Actions are pinned to commit SHAs. Whoever controls an action's repository can move a tag such as
@v4to any commit, and every workflow that uses it then runs the new code. A full SHA cannot be moved. Each pin is the commit the tag resolves to today, with the exact release in a comment:actions/checkout@v4resolves to)actions/setup-python@v5resolves to)actions/cache@v4resolves to)prefix-dev/setup-pixiactions/stale@v9resolves to)Lint runs pre-commit directly instead of through
pre-commit/action. That composite action callsactions/cache@v4by tag and installs an unpinnedpre-commit. The lint job now runs the same steps itself:pre-commit==4.7.0,actions/cachepinned by SHA with the same cache key, then the same three hooks.Lint and test run with a read-only token (
permissions: contents: read). The repository default gives workflows a token with write access, and neither workflow writes anything. The stale workflow already declares its own permissions.Checkouts don't persist the token (
persist-credentials: false), so no later step can read it from the working copy's git config. No step here pushes.What this does not cover:
.pre-commit-config.yamlstill names its hook repositories by tag (rev: v0.8.4etc.), and pre-commit fetches them at those tags.actionlintpasses on the three workflows. The codespell, ruff and ruff-format hooks pass locally withpre-commit==4.7.0.Related
github-actionsupdates will keep the SHA pins and their version comments current. Until then the config onmasterdoes not parse, so there are no bump PRs.release.yaml, so those files are not touched here. Both PRs edit the top oflint.yamlandtest.yaml. Thepermissionsblock sits right aftername:, so it merges cleanly with theworkflow_calltrigger that ci: release from annotated tags; remove CHANGELOG-archive.md #1588 adds; checked withgit merge-tree. When ci: release from annotated tags; remove CHANGELOG-archive.md #1588 calls these workflows throughworkflow_call, the read-only permissions are compatible with itscontents: readdefault. The actions inrelease.yamlwill be pinned in a follow-up once it merges.